feat: add `set_extrapolation_info` function in `morph.Morph` by ycexiao · Pull Request #255 · diffpy/diffpy.morph

ycexiao · 2025-09-11T19:17:08Z

What problem does this PR address?

address issues mentioned in #243.
Refactor the code to throw extrapolation warnings and throw an extrapolation warning for morph_shift.

What should the reviewer(s) do?

In this PR,

checkExtrapolation is implemented at morph.Morph to make it accessible to all derived classes.
Interfaces of related functions are changed to adopt checkExtrapolation's output.

codecov · 2025-09-11T19:19:35Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.92%. Comparing base (dce54cf) to head (2c9208a).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #255   +/-   ##
=======================================
  Coverage   99.92%   99.92%           
=======================================
  Files          23       23           
  Lines        1355     1355           
=======================================
  Hits         1354     1354           
  Misses          1        1

Files with missing lines	Coverage Δ
tests/test_morphsqueeze.py	`100.00% <100.00%> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ycexiao · 2025-09-11T19:26:25Z

tests/test_morphsqueeze.py

-    extrap_index_low = morph.extrap_index_low
-    extrap_index_high = morph.extrap_index_high
+
+    extrap_low = np.where(x_morph < min(x_squeezed))[0]


The code to compute extrap_index_*_expected and extrap_index_*_actual is identical, so we can remove the check and keep only one of them. In this case, extrap_index_*_actual is removed, which makes the implementation in the main program simpler.

Is there an argument to be made that this work could be done in checkExtrapolation()?

ycexiao · 2025-09-11T19:32:55Z

@sbillinge @Sparks29032, it's ready for review. Please check if this is okay, and then I will code the warnings for stretching and write the tests.

sbillinge

pls see comment

sbillinge · 2025-09-12T10:52:10Z

src/diffpy/morph/morph_io.py

+        cutoff_low = extrapolation_info["cutoff_low"]
+        cutoff_high = extrapolation_info["cutoff_high"]
+
+        if is_extrap_low or is_extrap_high:


This is getting better, but I still think it would be more readable with something like:

if low and high: low|high flow elif low: low flow elif high: high flow

sbillinge · 2025-09-12T10:53:46Z

src/diffpy/morph/morphs/morph.py

        return rv

+    def checkExtrapolation(self, x_true, x_extrapolate):
+        import numpy


is there a reason we are importing numpy locally? Put it at the top of the file?

No. I will put it at the top of the file.

sbillinge · 2025-09-12T10:54:41Z

src/diffpy/morph/morphs/morph.py

            ylabel(self.youtlabel)
        return rv

+    def checkExtrapolation(self, x_true, x_extrapolate):


make this a private function if it is only used within this module, or give it a docstring.

Also, just to chec before we put this in the base class, is this method called in multiple sub-classes?

Finally, the function name seems to be a bit off. It seems as if it is a setter, not checking anything. Something like set_extrapolation_info() or something like that.

The method will be called in morphsqueeze, morphshift, and morphstretch. I will add its docstring.

Yes, set_extrapolation_info will be better.

sbillinge · 2025-09-12T11:00:56Z

tests/test_morphsqueeze.py

-    extrap_index_low = morph.extrap_index_low
-    extrap_index_high = morph.extrap_index_high
+
+    extrap_low = np.where(x_morph < min(x_squeezed))[0]


Is there an argument to be made that this work could be done in checkExtrapolation()?

ycexiao · 2025-09-12T16:23:24Z

@sbillinge Is there an argument to be made that this work could be done in checkExtrapolation()?

Yes. We can add this to that function.

Sparks29032 · 2025-09-12T17:16:39Z

src/diffpy/morph/morph_io.py

+        if is_extrap_low or is_extrap_high:
+            if not is_extrap_high:
                wmsg = (
                    "Warning: points with grid value below "


Change "will be extrapolated" to "are extrapolated"

ycexiao

@sbillinge @Sparks29032 it's ready for review.

ycexiao · 2025-09-12T17:41:32Z

src/diffpy/morph/morph_io.py

+        cutoff_low = extrapolation_info["cutoff_low"]
+        cutoff_high = extrapolation_info["cutoff_high"]
+
+        if is_extrap_low and is_extrap_high:


Logic flow changed according to @sbillinge's comment.

ycexiao · 2025-09-12T17:42:18Z

src/diffpy/morph/morphs/morph.py

-"""
-
+"""Morph -- base class for defining a morph."""
+import numpy


import is moved to the top.

ycexiao · 2025-09-12T17:42:31Z

src/diffpy/morph/morphs/morph.py

        return rv

+    def set_extrapolation_info(self, x_true, x_extrapolate):
+        """Set extrapolation information of the concerned morphing


docstring is added.

ycexiao · 2025-09-12T17:42:46Z

src/diffpy/morph/morphs/morph.py

+        extrapolation_info = {
+            "is_extrap_low": is_extrap_low,
+            "cutoff_low": cutoff_low,
+            "extrap_index_low": extrap_index_low,


extrap_index_* is added.

ycexiao · 2025-09-12T17:44:50Z

src/diffpy/morph/morphs/morph.py

+            "cutoff_high": cutoff_high,
+            "extrap_index_high": extrap_index_high,
+        }
+        self.extrapolation_info = extrapolation_info


The way the function works has changed from returning a dict to setting the object's attributes since now the name starts with set.

ycexiao · 2025-09-12T17:45:58Z

tests/test_morphsqueeze.py

+    extrap_index_low_expected = extrap_low[-1] if extrap_low.size else 0
+    extrap_index_high_expected = extrap_high[0] if extrap_high.size else -1
+
+    extrapolation_info = morph.extrapolation_info


Getting the index now can be done in the morph.

ycexiao · 2025-09-12T17:47:26Z

tests/test_morphsqueeze.py

-    assert np.allclose(y_target_actual, y_target)
+    assert np.allclose(x_target_actual, x_target_expected)
+    assert np.allclose(y_target_actual, y_target_expected)
+    assert extrap_index_low_actual == extrap_index_low_expected


extrap_index_* assertions. are added

ycexiao · 2025-09-12T17:47:39Z

tests/test_morphsqueeze.py

            lambda x: (
                "Warning: points with grid value below "
-                f"{x[0]} will be extrapolated."
+                f"{x[0]} are extrapolated."


"will be" -> "are"

sbillinge · 2025-09-13T21:01:46Z

Nicely done, thanks @ycexiao !

feat: add checkExtrapolation function in morph.Morph

e35af9c

ycexiao commented Sep 11, 2025

View reviewed changes

ycexiao marked this pull request as ready for review September 11, 2025 19:29

sbillinge reviewed Sep 12, 2025

View reviewed changes

Sparks29032 reviewed Sep 12, 2025

View reviewed changes

refactor: rename the function to set_extrapolation_info

2c9208a

ycexiao changed the title ~~feat: add checkExtrapolation function in morph.Morph~~ feat: add set_extrapolation_info function in morph.Morph Sep 12, 2025

ycexiao commented Sep 12, 2025

View reviewed changes

sbillinge merged commit 2e5391d into diffpy:main Sep 13, 2025
5 checks passed

Sparks29032 mentioned this pull request Dec 19, 2025

Ensure no warning or errors when no extrapolation occurs #278

Merged

Conversation

ycexiao commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR address?

What should the reviewer(s) do?

Uh oh!

codecov bot commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ycexiao commented Sep 11, 2025

Uh oh!

sbillinge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ycexiao commented Sep 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ycexiao left a comment

Choose a reason for hiding this comment

Uh oh!

ycexiao Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sbillinge commented Sep 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ycexiao commented Sep 11, 2025 •

edited

Loading

codecov bot commented Sep 11, 2025 •

edited

Loading

ycexiao Sep 12, 2025 •

edited

Loading